perf: improve performance of isomorphicEncode #3101

tsctx · 2024-04-12T11:50:35Z

Benchmark Script

import { bench, group, run } from 'mitata'
import { isomorphicEncode } from '../../lib/web/fetch/util.js'

const characters =
  'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
const charactersLength = characters.length

function generateAsciiString (length) {
  let result = ''
  for (let i = 0; i < length; ++i) {
    result += characters[Math.floor(Math.random() * charactersLength)]
  }
  return result
}

const invalidIsomorphicEncodeValueRegex = /[^\x00-\xFF]/ // eslint-disable-line

function isomorphicEncode1 (input) {
  for (let i = 0; i < input.length; i++) {
    if (input.charCodeAt(i) > 0xff) {
      throw new TypeError('Unreachable')
    }
  }
  return input
}

function isomorphicEncode2 (input) {
  if (invalidIsomorphicEncodeValueRegex.test(input)) {
    throw new TypeError('Unreachable')
  }
  return input
}

const settings = {
  small: 10,
  middle: 30,
  long: 70
}

for (const [runName, length] of Object.entries(settings)) {
  const value = generateAsciiString(length);

  [
    { name: `${runName} (valid)`, value },
    {
      name: `${runName} (invalid)`,
      value: `${value.slice(0, -1)}${String.fromCharCode(0xff + 1)}`
    }
  ].forEach(({ name, value }) => {
    group(name, () => {
      [
        {
          name: 'original',
          fn: isomorphicEncode
        },
        {
          name: 'String#charCodeAt',
          fn: isomorphicEncode1
        },
        {
          name: 'RegExp#test',
          fn: isomorphicEncode2
        }
      ].forEach(({ name, fn }) => {
        bench(name, () => {
          try {
            return fn(value)
          } catch (err) {}
        })
      })
    })
  })
}

await run()

benchmark              time (avg)             (min … max)       p75       p99      p999
--------------------------------------------------------- -----------------------------
• small (valid)
--------------------------------------------------------- -----------------------------
original            56.48 ns/iter     (46.19 ns … 973 ns)  55.57 ns    122 ns    268 ns
String#charCodeAt   44.82 ns/iter     (33.79 ns … 875 ns)  57.47 ns    123 ns    324 ns
RegExp#test         39.23 ns/iter   (23.24 ns … 2'271 ns)  47.17 ns  88.53 ns    483 ns

summary for small (valid)
  RegExp#test
   1.14x faster than String#charCodeAt
   1.44x faster than original

• small (invalid)
--------------------------------------------------------- -----------------------------
original              162 µs/iter  (76'200 ns … 4'403 µs)    171 µs    425 µs  2'893 µs
String#charCodeAt  20'686 ns/iter  (11'300 ns … 5'352 µs) 25'200 ns 39'300 ns    204 µs
RegExp#test        15'999 ns/iter (10'136 ns … 35'269 ns) 20'371 ns 32'735 ns 35'269 ns

summary for small (invalid)
  RegExp#test
   1.29x faster than String#charCodeAt
   10.14x faster than original

• middle (valid)
--------------------------------------------------------- -----------------------------   
original              319 ns/iter     (160 ns … 7'139 ns)    334 ns  1'397 ns  7'139 ns
String#charCodeAt     202 ns/iter     (113 ns … 2'750 ns)    233 ns    463 ns  1'956 ns
RegExp#test         33.93 ns/iter   (23.19 ns … 2'237 ns)  34.57 ns    120 ns    285 ns

summary for middle (valid)
  RegExp#test
   5.96x faster than String#charCodeAt
   9.4x faster than original

• middle (invalid)
--------------------------------------------------------- -----------------------------   
original              116 µs/iter  (88'000 ns … 1'768 µs)    107 µs    274 µs    661 µs
String#charCodeAt  29'194 ns/iter (15'500 ns … 12'211 µs) 32'200 ns 55'800 ns    381 µs
RegExp#test        29'225 ns/iter  (16'100 ns … 3'467 µs) 33'000 ns 53'600 ns    221 µs

summary for middle (invalid)
  String#charCodeAt
   1x faster than RegExp#test
   3.96x faster than original

• long (valid)
--------------------------------------------------------- -----------------------------   
original              657 ns/iter     (352 ns … 3'251 ns)    748 ns  1'874 ns  3'251 ns
String#charCodeAt     307 ns/iter     (225 ns … 1'323 ns)    308 ns    630 ns  1'323 ns
RegExp#test         28.94 ns/iter     (23.63 ns … 230 ns)  25.63 ns    102 ns    156 ns

summary for long (valid)
  RegExp#test
   10.61x faster than String#charCodeAt
   22.72x faster than original

• long (invalid)
--------------------------------------------------------- -----------------------------   
original              155 µs/iter  (90'600 ns … 6'604 µs)    182 µs    338 µs  2'145 µs
String#charCodeAt  29'024 ns/iter  (16'500 ns … 3'502 µs) 33'500 ns 49'000 ns    212 µs
RegExp#test        25'481 ns/iter  (16'800 ns … 3'543 µs) 30'300 ns 47'300 ns    297 µs

summary for long (invalid)
  RegExp#test
   1.14x faster than String#charCodeAt
   6.09x faster than original

codecov-commenter · 2024-04-12T11:57:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.04%. Comparing base (65f768c) to head (81d853a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3101   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files          89       89           
  Lines       24333    24333           
=======================================
  Hits        22885    22885           
  Misses       1448     1448

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Uzlopak · 2024-04-12T12:11:59Z

interesting. I was also eyeing it, when I created #3091 . But one thing: I would maybe not write "Unreachable" as a message. Maybe if we have AssertionError, then use that. But yeah.

Uzlopak · 2024-04-12T12:15:08Z

Can you please not remove the benchmark?

tsctx · 2024-04-12T12:21:20Z

Can you please not remove the benchmark?

Sorry.

Uzlopak · 2024-04-12T12:30:01Z

Dont apologize, its fine.

Can you add a unit test for the error case? If we touch it we can immediatly improve the test coverage ;)

KhafraDev · 2024-04-12T13:59:58Z

why is the error case unreachable?

lib/web/fetch/util.js

mcollina

lgtm

perf: improve performance of isomorphicEncode

a16334d

tsctx force-pushed the perf/improve-performance-isomorphic-encode branch from c76954a to a16334d Compare April 12, 2024 12:01

tsctx added 2 commits April 12, 2024 21:05

minor change

17dd00d

chore: remove isomorphic-encode.mjs

0e6dc6c

ronag approved these changes Apr 12, 2024

View reviewed changes

create isomorphic-encode.mjs

adf1acb

tsctx commented Apr 12, 2024

View reviewed changes

lib/web/fetch/util.js Outdated Show resolved Hide resolved

apply suggestions from code review

baa0542

KhafraDev approved these changes Apr 12, 2024

View reviewed changes

tsctx commented Apr 12, 2024

View reviewed changes

lib/web/fetch/util.js Outdated Show resolved Hide resolved

apply suggestions from code review

81d853a

mcollina approved these changes Apr 12, 2024

View reviewed changes

mcollina merged commit ec74bf7 into nodejs:main Apr 12, 2024
26 checks passed

tsctx deleted the perf/improve-performance-isomorphic-encode branch April 12, 2024 20:54

Uzlopak mentioned this pull request Apr 13, 2024

assert is slow? #3091

Closed

github-actions bot mentioned this pull request Apr 22, 2024

[Release] v6.14.0 #3147

Merged

renovate bot mentioned this pull request Jul 22, 2024

fix(deps): update all non-major dependencies kirishima-ship/kirishima#40

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: improve performance of isomorphicEncode #3101

perf: improve performance of isomorphicEncode #3101

tsctx commented Apr 12, 2024 •

edited

Loading

codecov-commenter commented Apr 12, 2024 •

edited

Loading

Uzlopak commented Apr 12, 2024

Uzlopak commented Apr 12, 2024

tsctx commented Apr 12, 2024

Uzlopak commented Apr 12, 2024 •

edited

Loading

KhafraDev commented Apr 12, 2024

mcollina left a comment

perf: improve performance of isomorphicEncode #3101

perf: improve performance of isomorphicEncode #3101

Conversation

tsctx commented Apr 12, 2024 • edited Loading

codecov-commenter commented Apr 12, 2024 • edited Loading

Codecov Report

Uzlopak commented Apr 12, 2024

Uzlopak commented Apr 12, 2024

tsctx commented Apr 12, 2024

Uzlopak commented Apr 12, 2024 • edited Loading

KhafraDev commented Apr 12, 2024

mcollina left a comment

Choose a reason for hiding this comment

tsctx commented Apr 12, 2024 •

edited

Loading

codecov-commenter commented Apr 12, 2024 •

edited

Loading

Uzlopak commented Apr 12, 2024 •

edited

Loading